From 360e3376bf2d1cd460d849f5e316a5ccf975b344 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sun, 8 Jan 2012 08:40:00 +0000 Subject: [PATCH] In FileBackend: * Added getFileSize()/getFileStat() functions. Refactored some functions to use the stat function for better reuse and caching/consistency. * Refactored streamFile() to allow for subclasses to avoid local file copying with less duplication. Also make last-modified check actually work since we always get the timestamp of the original file. * Renamed 'ignoreErrors' parameter to 'force'. In FileBackendMultiWrite: * Simplified how read ops are done (use 'master' backend for consistency). * Added consistency check to doOperationsInternal() to check if the files are synced. * Various fixes after testing. In StreamFile: * Split out prepareForStream() function from stream() in StreamFile for code reuse. In FileBackendTest: * Properly cover FileBackendMultiWrite in tests. * Various test improvements. --- includes/StreamFile.php | 60 ++- includes/filerepo/FileRepo.php | 10 +- includes/filerepo/backend/FSFileBackend.php | 27 +- includes/filerepo/backend/FileBackend.php | 128 ++++-- .../backend/FileBackendMultiWrite.php | 262 +++++++------ includes/filerepo/backend/FileOp.php | 2 +- includes/filerepo/file/File.php | 2 +- languages/messages/MessagesEn.php | 1 + maintenance/language/messages.inc | 3 +- .../includes/filerepo/FileBackendTest.php | 368 +++++++++++++----- 10 files changed, 573 insertions(+), 290 deletions(-) diff --git a/includes/StreamFile.php b/includes/StreamFile.php index 8acbf9d606..89c862e1ad 100644 --- a/includes/StreamFile.php +++ b/includes/StreamFile.php @@ -5,6 +5,9 @@ * @file */ class StreamFile { + const READY_STREAM = 1; + const NOT_MODIFIED = 2; + /** * Stream a file to the browser, adding all the headings and fun stuff. * Headers sent include: Content-type, Content-Length, Last-Modified, @@ -16,17 +19,44 @@ class StreamFile { * @return bool Success */ public static function stream( $fname, $headers = array(), $sendErrors = true ) { - global $wgLanguageCode; - wfSuppressWarnings(); $stat = stat( $fname ); wfRestoreWarnings(); - if ( !$stat ) { + + $res = self::prepareForStream( $fname, $stat, $headers, $sendErrors ); + if ( $res == self::NOT_MODIFIED ) { + return true; // use client cache + } elseif ( $res == self::READY_STREAM ) { + return readfile( $fname ); + } else { + return false; // failed + } + } + + /** + * Call this function used in preparation before streaming a file. + * This function does the following: + * (a) sends Last-Modified, Content-type, and Content-Disposition headers + * (b) cancels any PHP output buffering and automatic gzipping of output + * (c) sends Content-Length header based on HTTP_IF_MODIFIED_SINCE check + * + * @param $path string Storage path or file system path + * @param $info Array File stat info with 'mtime' and 'size' fields + * @param $headers Array Additional headers to send + * @param $sendErrors bool Send error messages if errors occur (like 404) + * @return int|false READY_STREAM, NOT_MODIFIED, or false on failure + */ + public static function prepareForStream( + $path, array $info, $headers = array(), $sendErrors = true + ) { + global $wgLanguageCode; + + if ( !$info ) { if ( $sendErrors ) { header( 'HTTP/1.0 404 Not Found' ); header( 'Cache-Control: no-cache' ); header( 'Content-Type: text/html; charset=utf-8' ); - $encFile = htmlspecialchars( $fname ); + $encFile = htmlspecialchars( $path ); $encScript = htmlspecialchars( $_SERVER['SCRIPT_NAME'] ); echo "

File not found

@@ -38,12 +68,13 @@ class StreamFile { return false; } - header( 'Last-Modified: ' . gmdate( 'D, d M Y H:i:s', $stat['mtime'] ) . ' GMT' ); + // Sent Last-Modified HTTP header for client-side caching + header( 'Last-Modified: ' . wfTimestamp( TS_RFC2822, $info['mtime'] ) ); // Cancel output buffering and gzipping if set wfResetOutputBuffers(); - $type = self::getType( $fname ); + $type = self::getType( $path ); if ( $type && $type != 'unknown/unknown' ) { header( "Content-type: $type" ); } else { @@ -57,7 +88,7 @@ class StreamFile { } header( "Content-Disposition: inline;filename*=utf-8'$wgLanguageCode'" . - urlencode( basename( $fname ) ) ); + urlencode( basename( $path ) ) ); // Send additional headers foreach ( $headers as $header ) { @@ -67,26 +98,25 @@ class StreamFile { // Don't send if client has up to date cache if ( !empty( $_SERVER['HTTP_IF_MODIFIED_SINCE'] ) ) { $modsince = preg_replace( '/;.*$/', '', $_SERVER['HTTP_IF_MODIFIED_SINCE'] ); - $sinceTime = strtotime( $modsince ); - if ( $stat['mtime'] <= $sinceTime ) { + if ( wfTimestamp( TS_UNIX, $info['mtime'] ) <= strtotime( $modsince ) ) { ini_set( 'zlib.output_compression', 0 ); header( "HTTP/1.0 304 Not Modified" ); - return true; // ok + return self::NOT_MODIFIED; // ok } } - header( 'Content-Length: ' . $stat['size'] ); + header( 'Content-Length: ' . $info['size'] ); - return readfile( $fname ); + return self::READY_STREAM; // ok } /** * Determine the filetype we're dealing with - * @param $filename string - * @param $safe bool + * @param $filename string Storage path or file system path + * @param $safe bool Whether to do retroactive upload blacklist checks * @return null|string */ - private static function getType( $filename, $safe = true ) { + protected static function getType( $filename, $safe = true ) { global $wgTrivialMimeDetection; $ext = strrchr( $filename, '.' ); diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php index 94fc5cc344..4761509388 100644 --- a/includes/filerepo/FileRepo.php +++ b/includes/filerepo/FileRepo.php @@ -695,7 +695,7 @@ class FileRepo { } // Execute the store operation for each triplet - $opts = array( 'ignoreErrors' => true ); + $opts = array( 'force' => true ); if ( $flags & self::SKIP_LOCKING ) { $opts['nonLocking'] = true; } @@ -750,7 +750,7 @@ class FileRepo { } } // Actually delete files from storage... - $opts = array( 'ignoreErrors' => true ); + $opts = array( 'force' => true ); $this->backend->doOperations( $operations, $opts ); // Cleanup for disk source files... foreach ( $sourceFSFilesToDelete as $file ) { @@ -814,7 +814,7 @@ class FileRepo { // Delete the sources if required if ( $deleteOperations ) { - $opts = array( 'ignoreErrors' => true ); + $opts = array( 'force' => true ); $status->merge( $this->backend->doOperations( $deleteOperations, $opts ) ); } @@ -967,7 +967,7 @@ class FileRepo { } // Execute the operations for each triplet - $opts = array( 'ignoreErrors' => true ); + $opts = array( 'force' => true ); $status->merge( $backend->doOperations( $operations, $opts ) ); // Cleanup for disk source files... foreach ( $sourceFSFilesToDelete as $file ) { @@ -1102,7 +1102,7 @@ class FileRepo { // Move the files by execute the operations for each pair. // We're now committed to returning an OK result, which will // lead to the files being moved in the DB also. - $opts = array( 'ignoreErrors' => true ); + $opts = array( 'force' => true ); $status->merge( $backend->doOperations( $operations, $opts ) ); return $status; diff --git a/includes/filerepo/backend/FSFileBackend.php b/includes/filerepo/backend/FSFileBackend.php index edaf523b42..7c1cca4ed0 100644 --- a/includes/filerepo/backend/FSFileBackend.php +++ b/includes/filerepo/backend/FSFileBackend.php @@ -339,27 +339,28 @@ class FSFileBackend extends FileBackend { /** * @see FileBackend::doFileExists() */ - protected function doFileExists( array $params ) { + protected function doGetFileStat( array $params ) { list( $c, $source ) = $this->resolveStoragePathReal( $params['src'] ); if ( $source === null ) { return false; // invalid storage path } + wfSuppressWarnings(); - $exists = is_file( $source ); + if ( is_file( $source ) ) { // regular file? + $stat = stat( $source ); + } else { + $stat = false; + } wfRestoreWarnings(); - return $exists; - } - /** - * @see FileBackend::doGetFileTimestamp() - */ - public function doGetFileTimestamp( array $params ) { - list( $c, $source ) = $this->resolveStoragePathReal( $params['src'] ); - if ( $source === null ) { - return false; // invalid storage path + if ( $stat ) { + return array( + 'mtime' => wfTimestamp( TS_MW, $stat['mtime'] ), + 'size' => $stat['size'] + ); + } else { + return false; } - $fsFile = new FSFile( $source ); - return $fsFile->getTimestamp(); } /** diff --git a/includes/filerepo/backend/FileBackend.php b/includes/filerepo/backend/FileBackend.php index 2a8bb8d94f..19a6eddeeb 100644 --- a/includes/filerepo/backend/FileBackend.php +++ b/includes/filerepo/backend/FileBackend.php @@ -152,20 +152,20 @@ abstract class FileBackendBase { * contents as the new contents to be written there. * * $opts is an associative of boolean flags, including: - * 'ignoreErrors' : Errors that would normally cause a rollback do not. + * 'force' : Errors that would normally cause a rollback do not. * The remaining operations are still attempted if any fail. * 'nonLocking' : No locks are acquired for the operations. * This can increase performance for non-critical writes. - * This has no effect unless the 'ignoreErrors' flag is set. + * This has no effect unless the 'force' flag is set. * 'allowStale' : Don't require the latest available data. * This can increase performance for non-critical writes. - * This has no effect unless the 'ignoreErrors' flag is set. + * This has no effect unless the 'force' flag is set. * * Return value: * This returns a Status, which contains all warnings and fatals that occured * during the operation. The 'failCount', 'successCount', and 'success' members * will reflect each operation attempted. The status will be "OK" unless any - * of the operations failed and the 'ignoreErrors' parameter was not set. + * of the operations failed and the 'force' parameter was not set. * * @param $ops Array List of operations to execute in order * @param $opts Array Batch operation options @@ -175,7 +175,7 @@ abstract class FileBackendBase { if ( $this->readOnly != '' ) { return Status::newFatal( 'backend-fail-readonly', $this->name, $this->readOnly ); } - if ( empty( $opts['ignoreErrors'] ) ) { // sanity + if ( empty( $opts['force'] ) ) { // sanity unset( $opts['nonLocking'] ); unset( $opts['allowStale'] ); } @@ -371,6 +371,33 @@ abstract class FileBackendBase { */ abstract public function getFileContents( array $params ); + /** + * Get the size (bytes) of a file at a storage path in the backend. + * + * $params include: + * src : source storage path + * latest : use the latest available data + * + * @param $params Array + * @return integer|false Returns false on failure + */ + abstract public function getFileSize( array $params ); + + /** + * Get quick information about a file at a storage path in the backend. + * The result is an associative array that includes: + * mtime : the last-modified timestamp (TS_MW) or false + * size : the file size (bytes) or false + * + * $params include: + * src : source storage path + * latest : use the latest available data + * + * @param $params Array + * @return Array|false Returns false on failure + */ + abstract public function getFileStat( array $params ); + /** * Get a SHA-1 hash of the file at a storage path in the backend. * @@ -398,6 +425,7 @@ abstract class FileBackendBase { /** * Stream the file at a storage path in the backend. + * If the file does not exists, a 404 error will be given. * Appropriate HTTP headers (Status, Content-Type, Content-Length) * must be sent if streaming began, while none should be sent otherwise. * Implementations should flush the output buffer before sending data. @@ -807,43 +835,53 @@ abstract class FileBackend extends FileBackendBase { * @see FileBackendBase::fileExists() */ final public function fileExists( array $params ) { - $path = $params['src']; - if ( isset( $this->cache[$path]['exists'] ) ) { - return $this->cache[$path]['exists']; - } - $exists = $this->doFileExists( $params ); - if ( $exists ) { // don't cache negatives - $this->trimCache(); // limit memory - $this->cache[$path]['exists'] = $exists; + return (bool)$this->getFileStat( $params ); + } + + /** + * @see FileBackendBase::getFileTimestamp() + */ + final public function getFileTimestamp( array $params ) { + $stat = $this->getFileStat( $params ); + if ( $stat ) { + return $stat['mtime']; + } else { + return false; } - return $exists; } /** - * @see FileBackend::fileExists() + * @see FileBackendBase::getFileSize() */ - abstract protected function doFileExists( array $params ); + final public function getFileSize( array $params ) { + $stat = $this->getFileStat( $params ); + if ( $stat ) { + return $stat['size']; + } else { + return false; + } + } /** - * @see FileBackendBase::getFileTimestamp() + * @see FileBackendBase::getFileStat() */ - final public function getFileTimestamp( array $params ) { + final public function getFileStat( array $params ) { $path = $params['src']; - if ( isset( $this->cache[$path]['timestamp'] ) ) { - return $this->cache[$path]['timestamp']; + if ( isset( $this->cache[$path]['stat'] ) ) { + return $this->cache[$path]['stat']; } - $timestamp = $this->doGetFileTimestamp( $params ); - if ( $timestamp ) { // don't cache negatives + $stat = $this->doGetFileStat( $params ); + if ( is_array( $stat ) ) { // don't cache negatives $this->trimCache(); // limit memory - $this->cache[$path]['timestamp'] = $timestamp; + $this->cache[$path]['stat'] = $stat; } - return $timestamp; + return $stat; } /** - * @see FileBackend::getFileTimestamp() + * @see FileBackend::getFileStat() */ - abstract protected function doGetFileTimestamp( array $params ); + abstract protected function doGetFileStat( array $params ); /** * @see FileBackendBase::getFileContents() @@ -862,7 +900,7 @@ abstract class FileBackend extends FileBackendBase { /** * @see FileBackendBase::getFileSha1Base36() */ - public function getFileSha1Base36( array $params ) { + final public function getFileSha1Base36( array $params ) { $path = $params['src']; if ( isset( $this->cache[$path]['sha1'] ) ) { return $this->cache[$path]['sha1']; @@ -918,23 +956,39 @@ abstract class FileBackend extends FileBackendBase { /** * @see FileBackendBase::streamFile() */ - public function streamFile( array $params ) { + final public function streamFile( array $params ) { $status = Status::newGood(); - $fsFile = $this->getLocalReference( $params ); - if ( !$fsFile ) { + $info = $this->getFileStat( $params ); + if ( !$info ) { // let StreamFile handle the 404 + $status->fatal( 'backend-fail-notexists', $params['src'] ); + } + + // Set output buffer and HTTP headers for stream + $extraHeaders = $params['headers'] ? $params['headers'] : array(); + $res = StreamFile::prepareForStream( $params['src'], $info, $extraHeaders ); + if ( $res == StreamFile::NOT_MODIFIED ) { + // do nothing; client cache is up to date + } elseif ( $res == StreamFile::READY_STREAM ) { + $status = $this->doStreamFile( $params ); + } else { $status->fatal( 'backend-fail-stream', $params['src'] ); - return $status; } - $extraHeaders = isset( $params['headers'] ) - ? $params['headers'] - : array(); + return $status; + } + + /** + * @see FileBackend::streamFile() + */ + protected function doStreamFile( array $params ) { + $status = Status::newGood(); - $ok = StreamFile::stream( $fsFile->getPath(), $extraHeaders, false ); - if ( !$ok ) { + $fsFile = $this->getLocalReference( $params ); + if ( !$fsFile ) { + $status->fatal( 'backend-fail-stream', $params['src'] ); + } elseif ( !readfile( $fsFile->getPath() ) ) { $status->fatal( 'backend-fail-stream', $params['src'] ); - return $status; } return $status; diff --git a/includes/filerepo/backend/FileBackendMultiWrite.php b/includes/filerepo/backend/FileBackendMultiWrite.php index c9fafa53b9..b1bfd81c7e 100644 --- a/includes/filerepo/backend/FileBackendMultiWrite.php +++ b/includes/filerepo/backend/FileBackendMultiWrite.php @@ -12,22 +12,16 @@ * At least one of the backends must be declared the "master" backend. * * Only use this class when transitioning from one storage system to another. - * - * The order that the backends are defined sets the priority of which - * backend is read from or written to first. Functions like fileExists() - * and getFileProps() will return information based on the first backend - * that has the file. Special cases are listed below: - * a) getFileTimestamp() will always check only the master backend to - * avoid confusing and inconsistent results. - * - * All write operations are performed on all backends. + * + * Read operations are only done on the 'master' backend for consistency. + * All write operations are performed on all backends, in the order defined. * If an operation fails on one backend it will be rolled back from the others. * * @ingroup FileBackend */ class FileBackendMultiWrite extends FileBackendBase { /** @var Array Prioritized list of FileBackend objects */ - protected $fileBackends = array(); // array of (backend index => backends) + protected $backends = array(); // array of (backend index => backends) protected $masterIndex = -1; // index of master backend /** @@ -49,7 +43,7 @@ class FileBackendMultiWrite extends FileBackendBase { throw new MWException( 'No class given for a backend config.' ); } $class = $config['class']; - $this->fileBackends[$index] = new $class( $config ); + $this->backends[$index] = new $class( $config ); if ( !empty( $config['isMultiMaster'] ) ) { if ( $this->masterIndex >= 0 ) { throw new MWException( 'More than one master backend defined.' ); @@ -69,60 +63,114 @@ class FileBackendMultiWrite extends FileBackendBase { $status = Status::newGood(); $performOps = array(); // list of FileOp objects - $filesLockEx = $filesLockSh = array(); // storage paths to lock + $filesRead = $filesChanged = array(); // storage paths used // Build up a list of FileOps. The list will have all the ops // for one backend, then all the ops for the next, and so on. // These batches of ops are all part of a continuous array. - // Also build up a list of files to lock... - foreach ( $this->fileBackends as $index => $backend ) { - $backendOps = $this->substOpPaths( $ops, $backend ); + // Also build up a list of files read/changed... + foreach ( $this->backends as $index => $backend ) { + $backendOps = $this->substOpBatchPaths( $ops, $backend ); + // Add on the operation batch for this backend $performOps = array_merge( $performOps, $backend->getOperations( $backendOps ) ); - if ( $index == 0 && empty( $opts['nonLocking'] ) ) { - // Set "files to lock" from the first batch so we don't try to set all - // locks two or three times over (depending on the number of backends). - // A lock on one storage path is a lock on all the backends. + if ( $index == 0 ) { // first batch + // Get the files used for these operations. Each backend has a batch of + // the same operations, so we only need to get them from the first batch. foreach ( $performOps as $fileOp ) { - $filesLockSh = array_merge( $filesLockSh, $fileOp->storagePathsRead() ); - $filesLockEx = array_merge( $filesLockEx, $fileOp->storagePathsChanged() ); + $filesRead = array_merge( $filesRead, $fileOp->storagePathsRead() ); + $filesChanged = array_merge( $filesChanged, $fileOp->storagePathsChanged() ); } - // Optimization: if doing an EX lock anyway, don't also set an SH one - $filesLockSh = array_diff( $filesLockSh, $filesLockEx ); - // Lock the paths under the proxy backend's name - $this->unsubstPaths( $filesLockSh ); - $this->unsubstPaths( $filesLockEx ); + // Get the paths under the proxy backend's name + $this->unsubstPaths( $filesRead ); + $this->unsubstPaths( $filesChanged ); } } // Try to lock those files for the scope of this function... - $scopeLockS = $this->getScopedFileLocks( $filesLockSh, LockManager::LOCK_UW, $status ); - $scopeLockE = $this->getScopedFileLocks( $filesLockEx, LockManager::LOCK_EX, $status ); - if ( !$status->isOK() ) { - return $status; // abort + if ( empty( $opts['nonLocking'] ) ) { + $filesLockSh = array_diff( $filesRead, $filesChanged ); // optimization + $filesLockEx = $filesChanged; + $scopeLockS = $this->getScopedFileLocks( $filesLockSh, LockManager::LOCK_UW, $status ); + $scopeLockE = $this->getScopedFileLocks( $filesLockEx, LockManager::LOCK_EX, $status ); + if ( !$status->isOK() ) { + return $status; // abort + } } // Clear any cache entries (after locks acquired) - foreach ( $this->fileBackends as $backend ) { + foreach ( $this->backends as $backend ) { $backend->clearCache(); } + + // Do a consistency check to see if the backends agree + if ( count( $this->backends ) > 1 ) { + $status->merge( $this->consistencyCheck( array_merge( $filesRead, $filesChanged ) ) ); + if ( !$status->isOK() ) { + return $status; // abort + } + } + // Actually attempt the operation batch... $status->merge( FileOp::attemptBatch( $performOps, $opts ) ); return $status; } + /** + * Check that a set of files are consistent across all internal backends + * + * @param $paths Array + * @return Status + */ + public function consistencyCheck( array $paths ) { + $status = Status::newGood(); + + $mBackend = $this->backends[$this->masterIndex]; + foreach ( array_unique( $paths ) as $path ) { + $params = array( 'src' => $path ); + // Stat the file on the 'master' backend + $mStat = $mBackend->getFileStat( $this->substOpPaths( $params, $mBackend ) ); + // Check of all clone backends agree with the master... + foreach ( $this->backends as $index => $cBackend ) { + if ( $index === $this->masterIndex ) { + continue; // master + } + $cStat = $cBackend->getFileStat( $this->substOpPaths( $params, $cBackend ) ); + if ( $mStat ) { // file is in master + if ( !$cStat ) { // file should exist + $status->fatal( 'backend-fail-synced', $path ); + } elseif ( $cStat['size'] != $mStat['size'] ) { // wrong size + $status->fatal( 'backend-fail-synced', $path ); + } else { + $mTs = wfTimestamp( TS_UNIX, $mStat['mtime'] ); + $cTs = wfTimestamp( TS_UNIX, $cStat['mtime'] ); + if ( abs( $mTs - $cTs ) > 30 ) { // outdated file somewhere + $status->fatal( 'backend-fail-synced', $path ); + } + } + } else { // file is not in master + if ( $cStat ) { // file should not exist + $status->fatal( 'backend-fail-synced', $path ); + } + } + } + } + + return $status; + } + /** * Substitute the backend name in storage path parameters - * for a set of operations with a that of a given backend. + * for a set of operations with that of a given backend. * * @param $ops Array List of file operation arrays * @param $backend FileBackend * @return Array */ - protected function substOpPaths( array $ops, FileBackend $backend ) { + protected function substOpBatchPaths( array $ops, FileBackend $backend ) { $newOps = array(); // operations foreach ( $ops as $op ) { $newOp = $op; // operation - foreach ( array( 'src', 'srcs', 'dst' ) as $par ) { + foreach ( array( 'src', 'srcs', 'dst', 'dir' ) as $par ) { if ( isset( $newOp[$par] ) ) { $newOp[$par] = preg_replace( '!^mwstore://' . preg_quote( $this->name ) . '/!', @@ -136,6 +184,18 @@ class FileBackendMultiWrite extends FileBackendBase { return $newOps; } + /** + * Same as substOpBatchPaths() but for a single operation + * + * @param $op File operation array + * @param $backend FileBackend + * @return Array + */ + protected function substOpPaths( array $ops, FileBackend $backend ) { + $newOps = $this->substOpBatchPaths( array( $ops ), $backend ); + return $newOps[0]; + } + /** * Replace the backend part of storage paths with this backend's name * @@ -151,7 +211,7 @@ class FileBackendMultiWrite extends FileBackendBase { /** * @see FileBackendBase::prepare() */ - function prepare( array $params ) { + public function prepare( array $params ) { $status = Status::newGood(); foreach ( $this->backends as $backend ) { $realParams = $this->substOpPaths( $params, $backend ); @@ -163,7 +223,7 @@ class FileBackendMultiWrite extends FileBackendBase { /** * @see FileBackendBase::secure() */ - function secure( array $params ) { + public function secure( array $params ) { $status = Status::newGood(); foreach ( $this->backends as $backend ) { $realParams = $this->substOpPaths( $params, $backend ); @@ -175,7 +235,7 @@ class FileBackendMultiWrite extends FileBackendBase { /** * @see FileBackendBase::clean() */ - function clean( array $params ) { + public function clean( array $params ) { $status = Status::newGood(); foreach ( $this->backends as $backend ) { $realParams = $this->substOpPaths( $params, $backend ); @@ -187,134 +247,88 @@ class FileBackendMultiWrite extends FileBackendBase { /** * @see FileBackendBase::fileExists() */ - function fileExists( array $params ) { - # Hit all backends in case of failed operations (out of sync) - foreach ( $this->backends as $backend ) { - $realParams = $this->substOpPaths( $params, $backend ); - if ( $backend->fileExists( $realParams ) ) { - return true; - } - } - return false; + public function fileExists( array $params ) { + $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); + return $this->backends[$this->masterIndex]->fileExists( $realParams ); } /** * @see FileBackendBase::getFileTimestamp() */ - function getFileTimestamp( array $params ) { - // Skip non-master for consistent timestamps + public function getFileTimestamp( array $params ) { $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); return $this->backends[$this->masterIndex]->getFileTimestamp( $realParams ); } + /** + * @see FileBackendBase::getFileSize() + */ + public function getFileSize( array $params ) { + $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); + return $this->backends[$this->masterIndex]->getFileSize( $realParams ); + } + + /** + * @see FileBackendBase::getFileStat() + */ + public function getFileStat( array $params ) { + $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); + return $this->backends[$this->masterIndex]->getFileStat( $realParams ); + } + /** * @see FileBackendBase::getFileContents() */ - function getFileContents( array $params ) { - # Hit all backends in case of failed operations (out of sync) - foreach ( $this->backends as $backend ) { - $realParams = $this->substOpPaths( $params, $backend ); - $data = $backend->getFileContents( $realParams ); - if ( $data !== false ) { - return $data; - } - } - return false; + public function getFileContents( array $params ) { + $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); + return $this->backends[$this->masterIndex]->getFileContents( $realParams ); } /** * @see FileBackendBase::getFileSha1Base36() */ - function getFileSha1Base36( array $params ) { - # Hit all backends in case of failed operations (out of sync) - foreach ( $this->backends as $backend ) { - $realParams = $this->substOpPaths( $params, $backend ); - $hash = $backend->getFileSha1Base36( $realParams ); - if ( $hash !== false ) { - return $hash; - } - } - return false; + public function getFileSha1Base36( array $params ) { + $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); + return $this->backends[$this->masterIndex]->getFileSha1Base36( $realParams ); } /** * @see FileBackendBase::getFileProps() */ - function getFileProps( array $params ) { - # Hit all backends in case of failed operations (out of sync) - foreach ( $this->backends as $backend ) { - $realParams = $this->substOpPaths( $params, $backend ); - $props = $backend->getFileProps( $realParams ); - if ( $props !== null ) { - return $props; - } - } - return null; + public function getFileProps( array $params ) { + $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); + return $this->backends[$this->masterIndex]->getFileProps( $realParams ); } /** * @see FileBackendBase::streamFile() */ - function streamFile( array $params ) { - $status = Status::newGood(); - foreach ( $this->backends as $backend ) { - $realParams = $this->substOpPaths( $params, $backend ); - $subStatus = $backend->streamFile( $realParams ); - $status->merge( $subStatus ); - if ( $subStatus->isOK() ) { - // Pass isOK() despite fatals from other backends - $status->setResult( true ); - return $status; - } else { // failure - if ( headers_sent() ) { - return $status; // died mid-stream...so this is already fubar - } elseif ( strval( ob_get_contents() ) !== '' ) { - ob_clean(); // output was buffered but not sent; clear it - } - } - } - return $status; + public function streamFile( array $params ) { + $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); + return $this->backends[$this->masterIndex]->streamFile( $realParams ); } /** * @see FileBackendBase::getLocalReference() */ - function getLocalReference( array $params ) { - # Hit all backends in case of failed operations (out of sync) - foreach ( $this->backends as $backend ) { - $realParams = $this->substOpPaths( $params, $backend ); - $fsFile = $backend->getLocalReference( $realParams ); - if ( $fsFile ) { - return $fsFile; - } - } - return null; + public function getLocalReference( array $params ) { + $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); + return $this->backends[$this->masterIndex]->getLocalReference( $realParams ); } /** * @see FileBackendBase::getLocalCopy() */ - function getLocalCopy( array $params ) { - # Hit all backends in case of failed operations (out of sync) - foreach ( $this->backends as $backend ) { - $realParams = $this->substOpPaths( $params, $backend ); - $tmpFile = $backend->getLocalCopy( $realParams ); - if ( $tmpFile ) { - return $tmpFile; - } - } - return null; + public function getLocalCopy( array $params ) { + $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); + return $this->backends[$this->masterIndex]->getLocalCopy( $realParams ); } /** * @see FileBackendBase::getFileList() */ - function getFileList( array $params ) { - foreach ( $this->backends as $backend ) { - # Get results from the first backend - $realParams = $this->substOpPaths( $params, $backend ); - return $backend->getFileList( $realParams ); - } - return array(); // sanity + public function getFileList( array $params ) { + $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); + return $this->backends[$this->masterIndex]->getFileList( $realParams ); } } diff --git a/includes/filerepo/backend/FileOp.php b/includes/filerepo/backend/FileOp.php index c4bd7c9623..a43989079c 100644 --- a/includes/filerepo/backend/FileOp.php +++ b/includes/filerepo/backend/FileOp.php @@ -84,7 +84,7 @@ abstract class FileOp { $status = Status::newGood(); $allowStale = isset( $opts['allowStale'] ) && $opts['allowStale']; - $ignoreErrors = isset( $opts['ignoreErrors'] ) && $opts['ignoreErrors']; + $ignoreErrors = isset( $opts['force'] ) && $opts['force']; $predicates = FileOp::newPredicates(); // account for previous op in prechecks // Do pre-checks for each operation; abort on failure... foreach ( $performOps as $index => $fileOp ) { diff --git a/includes/filerepo/file/File.php b/includes/filerepo/file/File.php index 11b812ba7a..b5fe48168a 100644 --- a/includes/filerepo/file/File.php +++ b/includes/filerepo/file/File.php @@ -811,7 +811,7 @@ abstract class File { // overriding File::getThumbPath() to use a different zone (e.g. 'temp'). $status = $this->repo->getBackend()->store( array( 'src' => $tmpThumbPath, 'dst' => $thumbPath ), - array( 'ignoreErrors' => 1, 'nonLocking' => 1, 'allowStale' => 1 ) + array( 'force' => 1, 'nonLocking' => 1, 'allowStale' => 1 ) ); if ( $status->isOK() ) { $thumb->setStoragePath( $thumbPath ); diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index db172af42e..c32b9391c1 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -2254,6 +2254,7 @@ If the problem persists, contact an [[Special:ListUsers/sysop|administrator]].', 'backend-fail-read' => 'Could not read file $1.', 'backend-fail-create' => 'Could not create file $1.', 'backend-fail-readonly' => 'The backend "$1" is currently read-only. The reason given is: "$2"', +'backend-fail-synced' => 'The file "$1" is in an inconsistent state within the internal backends', # Lock manager 'lockmanager-notlocked' => 'Could not unlock "$1"; it is not locked.', diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc index dbd81f0af0..068f2bc66f 100644 --- a/maintenance/language/messages.inc +++ b/maintenance/language/messages.inc @@ -1363,7 +1363,8 @@ $wgMessageStructure = array( 'backend-fail-closetemp', 'backend-fail-read', 'backend-fail-create', - 'backend-fail-readonly' + 'backend-fail-readonly', + 'backend-fail-synced' ), 'lockmanager-errors' => array( diff --git a/tests/phpunit/includes/filerepo/FileBackendTest.php b/tests/phpunit/includes/filerepo/FileBackendTest.php index 3748deb09e..3f32891f5c 100644 --- a/tests/phpunit/includes/filerepo/FileBackendTest.php +++ b/tests/phpunit/includes/filerepo/FileBackendTest.php @@ -7,15 +7,16 @@ class FileBackendTest extends MediaWikiTestCase { function setUp() { parent::setUp(); - $this->backend = new FSFileBackend( array( + $tmpDir = wfTempDir() . '/' . time() . '-' . mt_rand(); + $this->singleBackend = new FSFileBackend( array( 'name' => 'localtesting', 'lockManager' => 'fsLockManager', 'containerPaths' => array( - 'cont1' => wfTempDir() . '/localtesting/cont1', - 'cont2' => wfTempDir() . '/localtesting/cont2' ) + 'cont1' => "$tmpDir/localtesting/cont1", + 'cont2' => "$tmpDir/localtesting/cont2" ) ) ); $this->multiBackend = new FileBackendMultiWrite( array( - 'name' => 'localtestingmulti', + 'name' => 'localtesting', 'lockManager' => 'fsLockManager', 'backends' => array( array( @@ -23,8 +24,8 @@ class FileBackendTest extends MediaWikiTestCase { 'class' => 'FSFileBackend', 'lockManager' => 'nullLockManager', 'containerPaths' => array( - 'cont1' => wfTempDir() . '/localtestingmulti1/cont1', - 'cont2' => wfTempDir() . '/localtestingmulti1/cont2' ), + 'cont1' => "$tmpDir/localtestingmulti1/cont1", + 'cont2' => "$tmpDir/localtestingmulti1/cont2" ), 'isMultiMaster' => false ), array( @@ -32,8 +33,8 @@ class FileBackendTest extends MediaWikiTestCase { 'class' => 'FSFileBackend', 'lockManager' => 'nullLockManager', 'containerPaths' => array( - 'cont1' => wfTempDir() . '/localtestingmulti2/cont1', - 'cont2' => wfTempDir() . '/localtestingmulti2/cont2' ), + 'cont1' => "$tmpDir/localtestingmulti2/cont1", + 'cont2' => "$tmpDir/localtestingmulti2/cont2" ), 'isMultiMaster' => true ) ) @@ -41,10 +42,14 @@ class FileBackendTest extends MediaWikiTestCase { $this->filesToPrune = $this->pathsToPrune = array(); } - private function singleBasePath() { + private function baseStorePath() { return 'mwstore://localtesting'; } + private function backendClass() { + return get_class( $this->backend ); + } + /** * @dataProvider provider_testStore */ @@ -52,29 +57,45 @@ class FileBackendTest extends MediaWikiTestCase { $this->filesToPrune[] = $source; $this->pathsToPrune[] = $dest; + $this->backend = $this->singleBackend; + $this->doTestStore( $op, $source, $dest ); + $this->tearDownFiles(); + + $this->backend = $this->multiBackend; + $this->doTestStore( $op, $source, $dest ); + $this->tearDownFiles(); + } + + function doTestStore( $op, $source, $dest ) { + $backendName = $this->backendClass(); + file_put_contents( $source, "Unit test file" ); $status = $this->backend->doOperation( $op ); + $this->assertEquals( array(), $status->errors, + "Store from $source to $dest succeeded without warnings ($backendName)." ); $this->assertEquals( true, $status->isOK(), - "Store from $source to $dest succeeded." ); - $this->assertEquals( true, $status->isGood(), - "Store from $source to $dest succeeded without warnings." ); + "Store from $source to $dest succeeded ($backendName)." ); $this->assertEquals( true, file_exists( $source ), - "Source file $source still exists." ); + "Source file $source still exists ($backendName)." ); $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $dest ) ), - "Destination file $dest exists." ); + "Destination file $dest exists ($backendName)." ); + + $this->assertEquals( filesize( $source ), + $this->backend->getFileSize( array( 'src' => $dest ) ), + "Destination file $dest has correct size ($backendName)." ); $props1 = FSFile::getPropsFromPath( $source ); $props2 = $this->backend->getFileProps( array( 'src' => $dest ) ); $this->assertEquals( $props1, $props2, - "Source and destination have the same props." ); + "Source and destination have the same props ($backendName)." ); } public function provider_testStore() { $cases = array(); $tmpName = TempFSFile::factory( "unittests_", 'txt' )->getPath(); - $toPath = $this->singleBasePath() . '/cont1/fun/obj1.txt'; + $toPath = $this->baseStorePath() . '/cont1/fun/obj1.txt'; $op = array( 'op' => 'store', 'src' => $tmpName, 'dst' => $toPath ); $cases[] = array( $op, // operation @@ -99,31 +120,49 @@ class FileBackendTest extends MediaWikiTestCase { $this->pathsToPrune[] = $source; $this->pathsToPrune[] = $dest; + $this->backend = $this->singleBackend; + $this->doTestCopy( $op, $source, $dest ); + $this->tearDownFiles(); + + $this->backend = $this->multiBackend; + $this->doTestCopy( $op, $source, $dest ); + $this->tearDownFiles(); + } + + function doTestCopy( $op, $source, $dest ) { + $backendName = $this->backendClass(); + $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => 'blahblah', 'dst' => $source ) ); - $this->assertEquals( true, $status->isOK(), "Creation of file at $source succeeded." ); + $this->assertEquals( true, $status->isOK(), + "Creation of file at $source succeeded ($backendName)." ); $status = $this->backend->doOperation( $op ); + $this->assertEquals( array(), $status->errors, + "Copy from $source to $dest succeeded without warnings ($backendName)." ); $this->assertEquals( true, $status->isOK(), - "Copy from $source to $dest succeeded." ); - $this->assertEquals( true, $status->isGood(), - "Copy from $source to $dest succeeded without warnings." ); + "Copy from $source to $dest succeeded ($backendName)." ); $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $source ) ), - "Source file $source still exists." ); + "Source file $source still exists ($backendName)." ); $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $dest ) ), - "Destination file $dest exists after copy." ); + "Destination file $dest exists after copy ($backendName)." ); + + $this->assertEquals( + $this->backend->getFileSize( array( 'src' => $source ) ), + $this->backend->getFileSize( array( 'src' => $dest ) ), + "Destination file $dest has correct size ($backendName)." ); $props1 = $this->backend->getFileProps( array( 'src' => $source ) ); $props2 = $this->backend->getFileProps( array( 'src' => $dest ) ); $this->assertEquals( $props1, $props2, - "Source and destination have the same props." ); + "Source and destination have the same props ($backendName)." ); } public function provider_testCopy() { $cases = array(); - $source = $this->singleBasePath() . '/cont1/file.txt'; - $dest = $this->singleBasePath() . '/cont2/fileMoved.txt'; + $source = $this->baseStorePath() . '/cont1/file.txt'; + $dest = $this->baseStorePath() . '/cont2/fileMoved.txt'; $op = array( 'op' => 'copy', 'src' => $source, 'dst' => $dest ); $cases[] = array( @@ -149,33 +188,51 @@ class FileBackendTest extends MediaWikiTestCase { $this->pathsToPrune[] = $source; $this->pathsToPrune[] = $dest; + $this->backend = $this->singleBackend; + $this->doTestMove( $op, $source, $dest ); + $this->tearDownFiles(); + + $this->backend = $this->multiBackend; + $this->doTestMove( $op, $source, $dest ); + $this->tearDownFiles(); + } + + public function doTestMove( $op, $source, $dest ) { + $backendName = $this->backendClass(); + $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => 'blahblah', 'dst' => $source ) ); - $this->assertEquals( true, $status->isOK(), "Creation of file at $source succeeded." ); + $this->assertEquals( true, $status->isOK(), + "Creation of file at $source succeeded ($backendName)." ); $status = $this->backend->doOperation( $op ); + $this->assertEquals( array(), $status->errors, + "Move from $source to $dest succeeded without warnings ($backendName)." ); $this->assertEquals( true, $status->isOK(), - "Move from $source to $dest succeeded." ); - $this->assertEquals( true, $status->isGood(), - "Move from $source to $dest succeeded without warnings." ); + "Move from $source to $dest succeeded ($backendName)." ); $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $source ) ), - "Source file $source does not still exists." ); + "Source file $source does not still exists ($backendName)." ); $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $dest ) ), - "Destination file $dest exists after move." ); + "Destination file $dest exists after move ($backendName)." ); + + $this->assertNotEquals( + $this->backend->getFileSize( array( 'src' => $source ) ), + $this->backend->getFileSize( array( 'src' => $dest ) ), + "Destination file $dest has correct size ($backendName)." ); $props1 = $this->backend->getFileProps( array( 'src' => $source ) ); $props2 = $this->backend->getFileProps( array( 'src' => $dest ) ); $this->assertEquals( false, $props1['fileExists'], - "Source file does not exist accourding to props." ); + "Source file does not exist accourding to props ($backendName)." ); $this->assertEquals( true, $props2['fileExists'], - "Destination file exists accourding to props." ); + "Destination file exists accourding to props ($backendName)." ); } public function provider_testMove() { $cases = array(); - $source = $this->singleBasePath() . '/cont1/file.txt'; - $dest = $this->singleBasePath() . '/cont2/fileMoved.txt'; + $source = $this->baseStorePath() . '/cont1/file.txt'; + $dest = $this->baseStorePath() . '/cont2/fileMoved.txt'; $op = array( 'op' => 'move', 'src' => $source, 'dst' => $dest ); $cases[] = array( @@ -200,31 +257,52 @@ class FileBackendTest extends MediaWikiTestCase { public function testDelete( $op, $source, $withSource, $okStatus ) { $this->pathsToPrune[] = $source; + $this->backend = $this->singleBackend; + $this->doTestDelete( $op, $source, $withSource, $okStatus ); + $this->tearDownFiles(); + + $this->backend = $this->multiBackend; + $this->doTestDelete( $op, $source, $withSource, $okStatus ); + $this->tearDownFiles(); + } + + public function doTestDelete( $op, $source, $withSource, $okStatus ) { + $backendName = $this->backendClass(); + if ( $withSource ) { $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => 'blahblah', 'dst' => $source ) ); - $this->assertEquals( true, $status->isOK(), "Creation of file at $source succeeded." ); + $this->assertEquals( true, $status->isOK(), + "Creation of file at $source succeeded ($backendName)." ); } $status = $this->backend->doOperation( $op ); if ( $okStatus ) { - $this->assertEquals( true, $status->isOK(), "Deletion of file at $source succeeded." ); + $this->assertEquals( array(), $status->errors, + "Deletion of file at $source succeeded without warnings ($backendName)." ); + $this->assertEquals( true, $status->isOK(), + "Deletion of file at $source succeeded ($backendName)." ); } else { - $this->assertEquals( false, $status->isOK(), "Deletion of file at $source failed." ); + $this->assertEquals( false, $status->isOK(), + "Deletion of file at $source failed ($backendName)." ); } $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $source ) ), - "Source file $source does not exist after move." ); + "Source file $source does not exist after move ($backendName)." ); + + $this->assertFalse( + $this->backend->getFileSize( array( 'src' => $source ) ), + "Source file $source has correct size (false) ($backendName)." ); $props1 = $this->backend->getFileProps( array( 'src' => $source ) ); - $this->assertEquals( false, $props1['fileExists'], - "Source file $source does not exist according to props." ); + $this->assertFalse( $props1['fileExists'], + "Source file $source does not exist according to props ($backendName)." ); } public function provider_testDelete() { $cases = array(); - $source = $this->singleBasePath() . '/cont1/myfacefile.txt'; + $source = $this->baseStorePath() . '/cont1/myfacefile.txt'; $op = array( 'op' => 'delete', 'src' => $source ); $cases[] = array( @@ -258,32 +336,55 @@ class FileBackendTest extends MediaWikiTestCase { public function testCreate( $op, $dest, $alreadyExists, $okStatus, $newSize ) { $this->pathsToPrune[] = $dest; + $this->backend = $this->singleBackend; + $this->doTestCreate( $op, $dest, $alreadyExists, $okStatus, $newSize ); + $this->tearDownFiles(); + + $this->backend = $this->multiBackend; + $this->doTestCreate( $op, $dest, $alreadyExists, $okStatus, $newSize ); + $this->tearDownFiles(); + } + + public function doTestCreate( $op, $dest, $alreadyExists, $okStatus, $newSize ) { + $backendName = $this->backendClass(); + $oldText = 'blah...blah...waahwaah'; if ( $alreadyExists ) { $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => $oldText, 'dst' => $dest ) ); - $this->assertEquals( true, $status->isOK(), "Creation of file at $dest succeeded." ); + $this->assertEquals( true, $status->isOK(), + "Creation of file at $dest succeeded ($backendName)." ); } $status = $this->backend->doOperation( $op ); if ( $okStatus ) { - $this->assertEquals( true, $status->isOK(), "Creation of file at $dest succeeded." ); + $this->assertEquals( array(), $status->errors, + "Creation of file at $dest succeeded without warnings ($backendName)." ); + $this->assertEquals( true, $status->isOK(), + "Creation of file at $dest succeeded ($backendName)." ); } else { - $this->assertEquals( false, $status->isOK(), "Creation of file at $dest failed." ); + $this->assertEquals( false, $status->isOK(), + "Creation of file at $dest failed ($backendName)." ); } $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $dest ) ), - "Dest file $dest exists after creation." ); + "Destination file $dest exists after creation ($backendName)." ); $props1 = $this->backend->getFileProps( array( 'src' => $dest ) ); $this->assertEquals( true, $props1['fileExists'], - "Dest file $dest exists according to props." ); + "Destination file $dest exists according to props ($backendName)." ); if ( $okStatus ) { // file content is what we saved $this->assertEquals( $newSize, $props1['size'], - "Dest file $dest has expected size according to props." ); + "Destination file $dest has expected size according to props ($backendName)." ); + $this->assertEquals( $newSize, + $this->backend->getFileSize( array( 'src' => $dest ) ), + "Destination file $dest has correct size ($backendName)." ); } else { // file content is some other previous text $this->assertEquals( strlen( $oldText ), $props1['size'], - "Dest file $dest has different size that given text according to props." ); + "Destination file $dest has original size according to props ($backendName)." ); + $this->assertEquals( strlen( $oldText ), + $this->backend->getFileSize( array( 'src' => $dest ) ), + "Destination file $dest has original size according to props ($backendName)." ); } } @@ -293,7 +394,7 @@ class FileBackendTest extends MediaWikiTestCase { public function provider_testCreate() { $cases = array(); - $source = $this->singleBasePath() . '/cont2/myspacefile.txt'; + $source = $this->baseStorePath() . '/cont2/myspacefile.txt'; $dummyText = 'hey hey'; $op = array( 'op' => 'create', 'content' => $dummyText, 'dst' => $source ); @@ -330,7 +431,20 @@ class FileBackendTest extends MediaWikiTestCase { */ public function testConcatenate( $op, $srcs, $srcsContent, $alreadyExists, $okStatus ) { $this->pathsToPrune = array_merge( $this->pathsToPrune, $srcs ); - $this->pathsToPrune[] = $op['dst']; + $this->filesToPrune[] = $op['dst']; + + $this->backend = $this->singleBackend; + $this->doTestConcatenate( $op, $srcs, $srcsContent, $alreadyExists, $okStatus ); + $this->tearDownFiles(); + + # FIXME + #$this->backend = $this->multiBackend; + #$this->doTestConcatenate( $op, $srcs, $srcsContent, $alreadyExists, $okStatus ); + #$this->tearDownFiles(); + } + + public function doTestConcatenate( $op, $srcs, $srcsContent, $alreadyExists, $okStatus ) { + $backendName = $this->backendClass(); $expContent = ''; // Create sources @@ -345,40 +459,49 @@ class FileBackendTest extends MediaWikiTestCase { } $status = $this->backend->doOperations( $ops ); - $this->assertEquals( true, $status->isOK(), "Creation of source files succeeded." ); + $this->assertEquals( true, $status->isOK(), + "Creation of source files succeeded ($backendName)." ); $dest = $op['dst']; if ( $alreadyExists ) { $ok = file_put_contents( $dest, 'blah...blah...waahwaah' ) !== false; - $this->assertEquals( true, $ok, "Creation of file at $dest succeeded." ); + $this->assertEquals( true, $ok, + "Creation of file at $dest succeeded ($backendName)." ); } else { $ok = file_put_contents( $dest, '' ) !== false; - $this->assertEquals( true, $ok, "Creation of 0-byte file at $dest succeeded." ); + $this->assertEquals( true, $ok, + "Creation of 0-byte file at $dest succeeded ($backendName)." ); } // Combine them $status = $this->backend->doOperation( $op ); if ( $okStatus ) { - $this->assertEquals( true, $status->isOK(), "Creation of concat file at $dest succeeded." ); + $this->assertEquals( array(), $status->errors, + "Creation of concat file at $dest succeeded without warnings ($backendName)." ); + $this->assertEquals( true, $status->isOK(), + "Creation of concat file at $dest succeeded ($backendName)." ); } else { - $this->assertEquals( false, $status->isOK(), "Creation of concat file at $dest failed." ); + $this->assertEquals( false, $status->isOK(), + "Creation of concat file at $dest failed ($backendName)." ); } if ( $okStatus ) { $this->assertEquals( true, is_file( $dest ), - "Dest concat file $dest exists after creation." ); + "Dest concat file $dest exists after creation ($backendName)." ); } else { $this->assertEquals( true, is_file( $dest ), - "Dest concat file $dest exists after failed creation." ); + "Dest concat file $dest exists after failed creation ($backendName)." ); } $contents = file_get_contents( $dest ); - $this->assertNotEquals( false, $contents, "File at $dest exists." ); + $this->assertNotEquals( false, $contents, "File at $dest exists ($backendName)." ); if ( $okStatus ) { - $this->assertEquals( $expContent, $contents, "Concat file at $dest has correct contents." ); + $this->assertEquals( $expContent, $contents, + "Concat file at $dest has correct contents ($backendName)." ); } else { - $this->assertNotEquals( $expContent, $contents, "Concat file at $dest has correct contents." ); + $this->assertNotEquals( $expContent, $contents, + "Concat file at $dest has correct contents ($backendName)." ); } } @@ -388,16 +511,16 @@ class FileBackendTest extends MediaWikiTestCase { $rand = mt_rand( 0, 2000000000 ) . time(); $dest = wfTempDir() . "/randomfile!$rand.txt"; $srcs = array( - $this->singleBasePath() . '/cont1/file1.txt', - $this->singleBasePath() . '/cont1/file2.txt', - $this->singleBasePath() . '/cont1/file3.txt', - $this->singleBasePath() . '/cont1/file4.txt', - $this->singleBasePath() . '/cont1/file5.txt', - $this->singleBasePath() . '/cont1/file6.txt', - $this->singleBasePath() . '/cont1/file7.txt', - $this->singleBasePath() . '/cont1/file8.txt', - $this->singleBasePath() . '/cont1/file9.txt', - $this->singleBasePath() . '/cont1/file10.txt' + $this->baseStorePath() . '/cont1/file1.txt', + $this->baseStorePath() . '/cont1/file2.txt', + $this->baseStorePath() . '/cont1/file3.txt', + $this->baseStorePath() . '/cont1/file4.txt', + $this->baseStorePath() . '/cont1/file5.txt', + $this->baseStorePath() . '/cont1/file6.txt', + $this->baseStorePath() . '/cont1/file7.txt', + $this->baseStorePath() . '/cont1/file8.txt', + $this->baseStorePath() . '/cont1/file9.txt', + $this->baseStorePath() . '/cont1/file10.txt' ); $content = array( 'egfage', @@ -425,7 +548,7 @@ class FileBackendTest extends MediaWikiTestCase { $op, // operation $srcs, // sources $content, // content for each source - true, // no dest already exists + true, // dest already exists false, // succeeds ); @@ -438,20 +561,38 @@ class FileBackendTest extends MediaWikiTestCase { public function testGetFileContents( $src, $content ) { $this->pathsToPrune[] = $src; + $this->backend = $this->singleBackend; + $this->doTestGetFileContents( $src, $content ); + $this->tearDownFiles(); + + $this->backend = $this->multiBackend; + $this->doTestGetFileContents( $src, $content ); + $this->tearDownFiles(); + } + + /** + * @dataProvider provider_testGetFileContents + */ + public function doTestGetFileContents( $src, $content ) { + $backendName = $this->backendClass(); + $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => $content, 'dst' => $src ) ); - $this->assertEquals( true, $status->isOK(), "Creation of file at $src succeeded." ); + $this->assertEquals( true, $status->isOK(), + "Creation of file at $src succeeded ($backendName)." ); $newContents = $this->backend->getFileContents( array( 'src' => $src ) ); - $this->assertNotEquals( false, $newContents, "Read of file at $src succeeded." ); + $this->assertNotEquals( false, $newContents, + "Read of file at $src succeeded ($backendName)." ); - $this->assertEquals( $content, $newContents, "Contents read match data at $src." ); + $this->assertEquals( $content, $newContents, + "Contents read match data at $src ($backendName)." ); } function provider_testGetFileContents() { $cases = array(); - $base = $this->singleBasePath(); + $base = $this->baseStorePath(); $cases[] = array( "$base/cont1/b/z/some_file.txt", "some file contents" ); $cases[] = array( "$base/cont1/b/some-other_file.txt", "more file contents" ); @@ -464,21 +605,35 @@ class FileBackendTest extends MediaWikiTestCase { public function testGetLocalCopy( $src, $content ) { $this->pathsToPrune[] = $src; + $this->backend = $this->singleBackend; + $this->doTestGetLocalCopy( $src, $content ); + $this->tearDownFiles(); + + $this->backend = $this->multiBackend; + $this->doTestGetLocalCopy( $src, $content ); + $this->tearDownFiles(); + } + + public function doTestGetLocalCopy( $src, $content ) { + $backendName = $this->backendClass(); + $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => $content, 'dst' => $src ) ); - $this->assertEquals( true, $status->isOK(), "Creation of file at $src succeeded." ); + $this->assertEquals( true, $status->isOK(), + "Creation of file at $src succeeded ($backendName)." ); $tmpFile = $this->backend->getLocalCopy( array( 'src' => $src ) ); - $this->assertNotNull( $tmpFile, "Creation of local copy of $src succeeded." ); + $this->assertNotNull( $tmpFile, + "Creation of local copy of $src succeeded ($backendName)." ); $contents = file_get_contents( $tmpFile->getPath() ); - $this->assertNotEquals( false, $contents, "Local copy of $src exists." ); + $this->assertNotEquals( false, $contents, "Local copy of $src exists ($backendName)." ); } function provider_testGetLocalCopy() { $cases = array(); - $base = $this->singleBasePath(); + $base = $this->baseStorePath(); $cases[] = array( "$base/cont1/a/z/some_file.txt", "some file contents" ); $cases[] = array( "$base/cont1/a/some-other_file.txt", "more file contents" ); @@ -491,21 +646,35 @@ class FileBackendTest extends MediaWikiTestCase { public function testGetLocalReference( $src, $content ) { $this->pathsToPrune[] = $src; + $this->backend = $this->singleBackend; + $this->doTestGetLocalReference( $src, $content ); + $this->tearDownFiles(); + + $this->backend = $this->multiBackend; + $this->doTestGetLocalReference( $src, $content ); + $this->tearDownFiles(); + } + + public function doTestGetLocalReference( $src, $content ) { + $backendName = $this->backendClass(); + $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => $content, 'dst' => $src ) ); - $this->assertEquals( true, $status->isOK(), "Creation of file at $src succeeded." ); + $this->assertEquals( true, $status->isOK(), + "Creation of file at $src succeeded ($backendName)." ); $tmpFile = $this->backend->getLocalReference( array( 'src' => $src ) ); - $this->assertNotNull( $tmpFile, "Creation of local copy of $src succeeded." ); + $this->assertNotNull( $tmpFile, + "Creation of local copy of $src succeeded ($backendName)." ); $contents = file_get_contents( $tmpFile->getPath() ); - $this->assertNotEquals( false, $contents, "Local copy of $src exists." ); + $this->assertNotEquals( false, $contents, "Local copy of $src exists ($backendName)." ); } function provider_testGetLocalReference() { $cases = array(); - $base = $this->singleBasePath(); + $base = $this->baseStorePath(); $cases[] = array( "$base/cont1/a/z/some_file.txt", "some file contents" ); $cases[] = array( "$base/cont1/a/some-other_file.txt", "more file contents" ); @@ -521,7 +690,19 @@ class FileBackendTest extends MediaWikiTestCase { // @TODO: testDoOperations public function testGetFileList() { - $base = $this->singleBasePath(); + $this->backend = $this->singleBackend; + $this->doTestGetFileList(); + $this->tearDownFiles(); + + $this->backend = $this->multiBackend; + $this->doTestGetFileList(); + $this->tearDownFiles(); + } + + public function doTestGetFileList() { + $backendName = $this->backendClass(); + + $base = $this->baseStorePath(); $files = array( "$base/cont1/test1.txt", "$base/cont1/test2.txt", @@ -546,7 +727,8 @@ class FileBackendTest extends MediaWikiTestCase { $ops[] = array( 'op' => 'create', 'content' => 'xxy', 'dst' => $file ); } $status = $this->backend->doOperations( $ops ); - $this->assertEquals( true, $status->isOK(), "Creation of files succeeded." ); + $this->assertEquals( true, $status->isOK(), + "Creation of files succeeded ($backendName)." ); // Expected listing $expected = array( @@ -575,7 +757,7 @@ class FileBackendTest extends MediaWikiTestCase { } sort( $list ); - $this->assertEquals( $expected, $list, "Correct file listing." ); + $this->assertEquals( $expected, $list, "Correct file listing ($backendName)." ); // Actual listing (with trailing slash) $list = array(); @@ -585,7 +767,7 @@ class FileBackendTest extends MediaWikiTestCase { } sort( $list ); - $this->assertEquals( $expected, $list, "Correct file listing." ); + $this->assertEquals( $expected, $list, "Correct file listing ($backendName)." ); foreach ( $files as $file ) { $this->backend->doOperation( array( 'op' => 'delete', 'src' => "$base/$file" ) ); @@ -595,16 +777,16 @@ class FileBackendTest extends MediaWikiTestCase { foreach ( $iter as $iter ) {} // no errors } - function tearDown() { - parent::tearDown(); + function tearDownFiles() { foreach ( $this->filesToPrune as $file ) { @unlink( $file ); } foreach ( $this->pathsToPrune as $file ) { $this->backend->doOperation( array( 'op' => 'delete', 'src' => $file ) ); - $this->multiBackend->doOperation( array( 'op' => 'delete', 'src' => $file ) ); } - $this->backend = $this->multiBackend = null; - $this->filesToPrune = $this->pathsToPrune = array(); + } + + function tearDown() { + parent::tearDown(); } } -- 2.20.1